Skip to content

feature: Enhanced Client Awareness (v5) #6537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jun 10, 2025

Conversation

calvincestari
Copy link
Member

@calvincestari calvincestari commented May 26, 2025

This adds support for the Enhanced Client Awareness request extensions; similar to apollographql/apollo-ios-dev#638 in Apollo iOS.

Note: This PR is against main which makes it part of a future v5 release.

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented May 26, 2025

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 2 changed, 0 removed
* (developer-tools)/kotlin/v5/advanced/compiler-plugins.mdx
* (developer-tools)/kotlin/v5/advanced/persisted-queries.mdx

Build ID: 8fb8ccfe3e3a5d14727ae11b

URL: https://www.apollographql.com/docs/deploy-preview/8fb8ccfe3e3a5d14727ae11b

Copy link
Member Author

@calvincestari calvincestari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinbonnin, @BoD - is this PR on the right track so far?

@calvincestari
Copy link
Member Author

I'm not sure what the :apollo-api:jvmApiCheck is that's failing. We'll have to talk tomorrow..

@calvincestari
Copy link
Member Author

I cancelled the tests-integration check, it looked like it was taking far too long.

Copy link
Contributor

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 🚀

@calvincestari calvincestari force-pushed the feature/enhanced-client-awareness branch from a4e432f to f4fc35d Compare June 2, 2025 21:36
@calvincestari calvincestari marked this pull request as ready for review June 6, 2025 01:50
@calvincestari
Copy link
Member Author

@martinbonnin / @BoD - I think this is now ready for another review.

}

if (sendEnhancedClientAwarenessExtensions) {
name("clientLibrary")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a reference that we could link as a comment here for future readers that would like more context for the feature?

@calvincestari
Copy link
Member Author

OK, I believe this is now all done. 🎉

I think the only thing left to discuss about it is whether we want v5 or v4 + v5?

@calvincestari
Copy link
Member Author

calvincestari commented Jun 6, 2025

I've noticed that the tests-integration step appears stuck. I looked at the log and could not see any error so I'm not sure what the delay was - is this something that you've seen before?

@calvincestari
Copy link
Member Author

I've noticed that the tests-integration step appears stuck. I looked at the log and could not see any error so I'm not sure what the delay was - is this something that you've seen before?

OK, I see what the issue is; there are some test failures but the logs do not make that obvious. Looks like the test fixtures need to be updated or this feature disabled in those tests. Here's an example:

test.LoggingInterceptorTest.levelBody[macosArm64] FAILED
    kotlin.AssertionError: Expected <post http://0.0.0.0/
    accept: multipart/mixed;deferspec=20220824, application/graphql-response+json, application/json
    [end of headers]
    {"operationname":"heroname","variables":{},"query":"query heroname { hero { name } }"}

    >, actual <post http://0.0.0.0/
    accept: multipart/mixed;deferspec=20220824, application/graphql-response+json, application/json
    [end of headers]
    {"operationname":"heroname","variables":{},"query":"query heroname { hero { name } }","extensions":{"clientlibrary":{"name":"apollo-kotlin","version":"5.0.0-snapshot"}}}

@apollo-librarian
Copy link

apollo-librarian bot commented Jun 6, 2025

✅ Docs preview ready

The preview is ready to be viewed. View the preview

File Changes

0 new, 5 changed, 0 removed
* (developer-tools)/apollo-mcp-server/(latest)/command-reference.mdx
* (developer-tools)/kotlin/v5/advanced/compiler-plugins.mdx
* (developer-tools)/kotlin/v5/advanced/persisted-queries.mdx
* (developer-tools)/rover/commands/dev.mdx
* graphos/schema-design/federated-schemas/reference/moving-to-federation-2.mdx

Build ID: eb2155dce39d5e5df37fc967

URL: https://www.apollographql.com/docs/deploy-preview/eb2155dce39d5e5df37fc967

@martinbonnin
Copy link
Contributor

Looks like the test fixtures need to be updated

Indeed, tests updated in 611b459

I've noticed that the tests-integration step appears stuck

This is usually when the JS test fail. The promise never returns and the test just hangs. Not 100% sure how to fix it.

@calvincestari
Copy link
Member Author

@martinbonnin - I think in this case it might be better to turn off the feature because of the version string; the tests will all need to be updated each time a new version is released.

@calvincestari
Copy link
Member Author

Oh wait..that was your commit updating them.

@martinbonnin
Copy link
Contributor

I'd like to avoid those tests doing too much configuration. The drawback is that now the expected string is computed based on the current version instead of before it was a plain string literal but I think this is OKay.

@martinbonnin
Copy link
Contributor

This is usually when the JS test fail

I just tested again by simulating an explicit failure and it worked well (after 2min still...). There is definitely something fishy in the JS test code but since this JS is not the highest used feature, I haven't taken the time to dive deep into this. Mid term I'm hoping that https://youtrack.jetbrains.com/issue/KT-58205/Kotlin-Gradle-Plugin-make-multiplatform-tests-build-cacheable makes this mostly indolore 🤞

@calvincestari
Copy link
Member Author

Thanks for helping fix it. ❤️

@calvincestari calvincestari changed the title feature: Enhanced Client Awareness feature: Enhanced Client Awareness (v5) Jun 9, 2025
{"operationName":"MultipleUpload","variables":{"files":[null,null]},"query":"mutation MultipleUpload($files: [Upload!]!) { multipleUpload(files: $files) { id path filename mimetype } }","extensions":{"clientLibrary":{"name":"apollo-kotlin","version":"5.0.0-SNAPSHOT"}}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the release.main.kts script to take care of this

Copy link
Contributor

@BoD BoD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

@martinbonnin martinbonnin merged commit 6aa22dc into main Jun 10, 2025
7 checks passed
@martinbonnin martinbonnin deleted the feature/enhanced-client-awareness branch June 10, 2025 15:16
@@ -243,6 +244,18 @@ fun setVersionInIntelliJPlugin(version: String) {
}
}

fun setVersionInFixtures(nextSnapshot: String) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants